Skip to content

Fix radio, checkbox and select #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 30, 2019
Merged

Fix radio, checkbox and select #55

merged 7 commits into from
Jul 30, 2019

Conversation

afontcu
Copy link
Member

@afontcu afontcu commented Jul 8, 2019

AFAIK, checkbox and radio elements use a checked property to express that they've been selected, which is set to true. So there's no need to pass a value argument.

Same thing with <option> elements. There was an issue with this (due to copy&paste, I guess! 😇), so I also improved naming a bit.

I also improved tests a bit, to demonstrate that the updated element in each case is properly being checked.

@afontcu afontcu requested a review from dfcook July 8, 2019 07:34
@afontcu afontcu changed the title Fix radio and checkbox Fix radio, checkbox and select Jul 17, 2019
@afontcu
Copy link
Member Author

afontcu commented Jul 17, 2019

btw, I tried to update to the newest version of jest-dom/extend-expect, which includes a .toHaveValue(), but I couldn't make tests pass. I'm not sure if it's on us or on them, but tests look fine right now... 🤔

@afontcu afontcu requested review from dfcook and removed request for dfcook July 17, 2019 19:15
@afontcu afontcu force-pushed the fix-radio-checkbox branch from 35c4fdb to a50ec54 Compare July 20, 2019 16:46
@dfcook dfcook merged commit 45f8654 into master Jul 30, 2019
@dfcook dfcook deleted the fix-radio-checkbox branch July 30, 2019 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants